Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Updated nvidia docker base image to CUDA 10.0 #12850

Closed
wants to merge 4 commits into from
Closed

Updated nvidia docker base image to CUDA 10.0 #12850

wants to merge 4 commits into from

Conversation

lebeg
Copy link
Contributor

@lebeg lebeg commented Oct 17, 2018

Description

PR requires newly deployed AMIs with updated drivers.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Feature1, tests, (and when applicable, API doc)
  • Feature2, tests, (and when applicable, API doc)

Comments

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

@lebeg lebeg mentioned this pull request Oct 17, 2018
Closed
@Roshrini
Copy link
Member

@mxnet-label-bot [Docker, pr-awaiting-review]

@marcoabreu marcoabreu added Docker pr-awaiting-review PR is waiting for code review labels Oct 17, 2018
@KellenSunderland
Copy link
Contributor

Looks like a contrib operator fails to build for the updated CentOS image (maybe a slightly newer compiler with an additional warning which is treated as an error)

const std::vector<mxnet::TBlob>&, const std::vector<mxnet::TBlob>&, const std::vector<mxnet::TBlob>&, const std::vector<mxnet::OpReqType>&, const std::vector<mxnet::TBlob>&, const std::vector<mxnet::TBlob>&) [with xpu = mshadow::gpu; DType = float]':
src/operator/contrib/./deformable_psroi_pooling-inl.h:153:34: error: 'trans.mshadow::Tensor<mshadow::gpu, 4, float>::dptr_' may be used uninitialized in this function [-Werror=maybe-uninitialized]
cc1plus: all warnings being treated as errors

@ankkhedia
Copy link
Contributor

@lebeg Could you please take a look into the CI failures.

@lebeg lebeg mentioned this pull request Oct 31, 2018
Open
@lebeg lebeg force-pushed the update_cuda branch 3 times, most recently from 0ba6f12 to 0efaebb Compare November 7, 2018 14:39
@lebeg lebeg changed the title Updated nvidia docker base image to CUDA 9.2 Updated nvidia docker base image to CUDA 10.0 Nov 8, 2018
@lebeg lebeg requested a review from szha as a code owner November 8, 2018 13:57
@lebeg lebeg mentioned this pull request Nov 8, 2018
Closed
larroy
larroy suggested changes Nov 8, 2018
View reviewed changes
Makefile Outdated
@@ -79,7 +79,7 @@ WARNFLAGS= -Wall -Wsign-compare
CFLAGS = -DMSHADOW_FORCE_STREAM $(WARNFLAGS)

ifeq ($(DEV), 1)
CFLAGS += -g -Werror
CFLAGS += -g -Werror -fno-var-tracking-assignments
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we try to use a #pragma directly on the code instead of disabling for all platforms in the makefile :-( ?

ci/docker/Dockerfile.build.centos7_gpu
@@ -18,7 +18,7 @@
#
# Dockerfile to build and run MXNet on CentOS 7 for GPU

FROM nvidia/cuda:9.1-cudnn7-devel-centos7
FROM nvidia/cuda:10.0-cudnn7-devel-centos7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@larroy
Copy link
Contributor

larroy commented Nov 8, 2018

One of our users asked me about Cuda10, this is great. But I guess we would also need a pip package in the deployment side, right @szha?

@szha
Copy link
Member

szha commented Nov 8, 2018

@larroy yes. I think what I can try to do first is to publish the nightly to a publicly accessible s3 repo, so that at least the distribution can start.

@lebeg lebeg force-pushed the update_cuda branch 2 times, most recently from 72c7599 to 9c7ce21 Compare November 9, 2018 16:14
@lebeg
Copy link
Contributor Author

lebeg commented Nov 9, 2018

Moved centos7_gpu build to cmake.

@lebeg lebeg force-pushed the update_cuda branch 2 times, most recently from c7228f0 to c743127 Compare November 12, 2018 15:28
larroy
larroy suggested changes Nov 12, 2018
View reviewed changes
Copy link
Contributor

@larroy larroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer having a pragma as commented. Can you please check CI failures?

@vandanavk
Copy link
Contributor

@mxnet-label-bot update [pr-awaiting-testing, Docker]

@marcoabreu marcoabreu added pr-awaiting-testing PR is reviewed and waiting CI build and test and removed pr-awaiting-review PR is waiting for code review labels Nov 26, 2018
@lebeg
Copy link
Contributor Author

lebeg commented Nov 28, 2018

I don't have time for this right now, will reopen it back once it will be ready.

@lebeg lebeg closed this Nov 28, 2018
@lebeg lebeg mentioned this pull request Mar 25, 2019
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@larroy larroy larroy requested changes

@KellenSunderland KellenSunderland KellenSunderland approved these changes

@szha szha Awaiting requested review from szha szha is a code owner

Assignees
No one assigned
Labels
Docker pr-awaiting-testing PR is reviewed and waiting CI build and test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@lebeg @Roshrini @KellenSunderland @ankkhedia @larroy @szha @vandanavk @marcoabreu